Skip to content

Conversation

@martonvago
Copy link
Contributor

@martonvago martonvago commented Nov 4, 2025

Description

This PR handles grouped errors at $.resources[x].schema.fields[x].constraints.enum. It's the worst one yet!

This schema node is deeply nested, so handling errors here is a little more complex.

The situation is the following: for each field, you can list the allowed values for that field under "constraints": {"enum": [...]}. Depending on the type of the field, the type of the values in enum can be different. Sometimes it's straightforward (e.g. for string fields, enum values have to be strings), but sometimes the schema for enum is more permissive (e.g. for number fields, enum values can be either numbers or strings). However, you cannot mix types.

If enum has mixed types or if some of the values are the wrong type, all sub-schemas under enum flag errors. These are usually contradictory, e.g., for a number field [1, "inf"] will alert both because 1 is not a string and because "inf" is not a number...

The logic in the function:

  • if the error is not even for the current field type -> remove all errors
  • if there are any errors on the enum property itself (not an array, duplicate items, etc.) -> keep only these
  • the remaining errors are all about the individual values in the enum array being the wrong type
    • if the values are not all the same type -> flag only this
    • if they are all the same type -> flag that this is the wrong type

Part of #15

Needs an in-depth review.

Checklist

  • Formatted Markdown
  • Ran just run-all

@martonvago martonvago self-assigned this Nov 4, 2025
@martonvago martonvago moved this from Todo to In Progress in Iteration planning Nov 4, 2025
@martonvago martonvago moved this from In Progress to In Review in Iteration planning Nov 6, 2025
@martonvago martonvago marked this pull request as ready for review November 6, 2025 14:17
@martonvago martonvago requested a review from a team as a code owner November 6, 2025 14:17
Copy link
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat! Just some fairly small comments ☺️

@github-project-automation github-project-automation bot moved this from In Review to In Progress in Iteration planning Nov 7, 2025
@martonvago martonvago moved this from In Progress to In Review in Iteration planning Nov 7, 2025
@martonvago martonvago requested a review from lwjohnst86 November 7, 2025 12:02
Copy link
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Some fairly small comments 👍

@github-project-automation github-project-automation bot moved this from In Review to In Progress in Iteration planning Nov 14, 2025
@martonvago martonvago moved this from In Progress to In Review in Iteration planning Nov 14, 2025
Copy link
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor suggestion

@github-project-automation github-project-automation bot moved this from In Review to In Progress in Iteration planning Nov 14, 2025
@lwjohnst86
Copy link
Member

@martonvago plus resolving conflicts ☺️

@martonvago martonvago moved this from In Progress to In Review in Iteration planning Nov 14, 2025
@github-project-automation github-project-automation bot moved this from In Review to In Progress in Iteration planning Nov 14, 2025
@lwjohnst86 lwjohnst86 merged commit ab276df into main Nov 14, 2025
6 checks passed
@lwjohnst86 lwjohnst86 deleted the feat/grouped-error-constraint-enum branch November 14, 2025 13:18
@github-project-automation github-project-automation bot moved this from In Progress to Done in Iteration planning Nov 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants